Skip to content

deleted settings panel, move logical to use popover#171

Open
DinaLaptii wants to merge 2 commits intoNVIDIA-AI-Blueprints:developfrom
DinaLaptii:fix/ui-design
Open

deleted settings panel, move logical to use popover#171
DinaLaptii wants to merge 2 commits intoNVIDIA-AI-Blueprints:developfrom
DinaLaptii:fix/ui-design

Conversation

@DinaLaptii
Copy link
Copy Markdown
Contributor

I think we need a feature toggle for these changes. I have updated the design according to the Figma:
https://www.figma.com/design/JHtiVv7hEmntIYISMcoQJP/AI-Q-3.0--Nvidia-Agent-Space-?node-id=139-7444&t=UdqFIqYeUipeSgRe-0

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 5, 2026

Greptile Summary

This PR deletes the standalone SettingsPanel right-side panel and moves the appearance/theme toggle control into the user avatar dropdown popover, making theme switching accessible in-context for both authenticated users and the default-user (auth-disabled) flow. A new AppearanceThemeControl segmented control replaces the previous Select-based theme picker, and a Documentation button with an external-link indicator replaces the old Settings button in the AppBar.

  • Theme control is now embedded directly in both UserDropdownContent and AuthDisabledContent popovers, with role="radiogroup" / role="radio" ARIA markup; theme application continues to work via the existing useThemeEffect hook in providers.tsx
  • SettingsPanel is removed from MainLayout rendering and its import is dropped; the file itself (SettingsPanel.tsx) and its spec remain on disk as orphaned dead code
  • RightPanelType in types.ts still includes 'settings' which is now an unreachable dead value
  • Tests are updated throughout to reflect the new "open documentation" button label and radiogroup presence; the settings-panel open test is correctly removed
  • The OpenExternal icon is added to the icon adapter for the new Documentation button

Confidence Score: 5/5

Safe to merge; all remaining findings are minor cleanup items (P2) that do not affect runtime behaviour.

The core functionality is correctly migrated — theme control was already wired to providers.tsx which reads from the layout store and applies CSS classes to the document. Tests are updated. The two remaining items (stale RightPanelType union member, orphaned SettingsPanel files) are non-blocking style/cleanup issues.

frontends/ui/src/features/layout/types.ts (stale 'settings' in RightPanelType) and the undeleted SettingsPanel.tsx / SettingsPanel.spec.tsx files.

Important Files Changed

Filename Overview
frontends/ui/src/adapters/ui/icons.tsx Adds OpenExternal icon export — minimal, clean addition.
frontends/ui/src/features/layout/components/AppBar.tsx Removes Settings button, adds Documentation link, and introduces AppearanceThemeControl in both user dropdown popovers; stale 'settings' type and orphaned SettingsPanel files remain as cleanup work.
frontends/ui/src/features/layout/components/AppBar.spec.tsx Updates tests to reflect the new Documentation button and theme radiogroup presence; removes the now-obsolete settings-panel open test.
frontends/ui/src/features/layout/components/MainLayout.tsx Removes SettingsPanel import and rendering — straightforward, correct cleanup.
frontends/ui/src/features/layout/components/MainLayout.spec.tsx Removes SettingsPanel mock to match the implementation change.

Sequence Diagram

sequenceDiagram
    participant User
    participant AppBar
    participant Popover
    participant AppearanceThemeControl
    participant LayoutStore
    participant ThemeProvider

    User->>AppBar: Click avatar button
    AppBar->>Popover: Open user dropdown
    Popover->>AppearanceThemeControl: Render theme segments (System/Dark/Light)
    User->>AppearanceThemeControl: Click Dark / Light / System
    AppearanceThemeControl->>LayoutStore: setTheme(mode)
    LayoutStore-->>ThemeProvider: theme state update (via useLayoutStore)
    ThemeProvider->>ThemeProvider: Apply nv-dark / nv-light class to document root
Loading

Reviews (2): Last reviewed commit: "fix tests" | Re-trigger Greptile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant